Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix configuration of breaks when using recent scales package. #140

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

plietar
Copy link

@plietar plietar commented Feb 19, 2024

The latest release of the scales package has added a pair of optional arguments, d_transform and d_inverse, in the middle of the parameter list for trans_new (see r-lib/scales#341). As a consequence, this has shifted the position of the breaks parameter from 4th to 6th.

Because the bench_time_trans and bench_bytes_trans functions from this package were passing in the breaks object positionally, this change in the scales package means that the breaks object is now matched with the new d_transform formal parameter and it ends up being ignored.

This commit changes the arguments to trans_new from being positional to being named, which should be more robust to future changes to the function.

The latest release of the scales package has added a pair of optional
arguments, `d_transform` and `d_inverse`, in the middle of the parameter
list for `trans_new` (see r-lib/scales#341). As
a consequence, this has shifted the position of the `breaks` parameter
from 4th to 6th.

Because the `bench_time_trans` and `bench_bytes_trans` functions from
this package were passing in the breaks object positionally, this change
in the scales package means that the breaks object is now matched with
the new `d_transform` formal parameter and it ends up being ignored.

This commit changes the arguments to `trans_new` from being positional
to being named, which should be more robust to future changes to the
function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant